Skip to content

fix(harness): stream Docker exec commands through stdin#2148

Open
richuV wants to merge 1 commit into
agentscope-ai:mainfrom
richuV:codex/docker-stdin-exec-2135
Open

fix(harness): stream Docker exec commands through stdin#2148
richuV wants to merge 1 commit into
agentscope-ai:mainfrom
richuV:codex/docker-stdin-exec-2135

Conversation

@richuV

@richuV richuV commented Jul 12, 2026

Copy link
Copy Markdown

Summary

  • run Docker sandbox shell programs through docker exec -i ... sh -s
  • keep shell text out of the host process argument list
  • preserve UTF-8 complex commands and avoid Windows command-line length limits
  • add unit contracts plus an opt-in real Docker regression test

Why

docker exec ... sh -c <command> puts the complete shell program in the Windows CreateProcess argument list. Complex nested quoting can be corrupted, and large filesystem scripts can exceed the host command-line limit.

The public Sandbox API and command semantics remain unchanged; only the Docker CLI transport changes.

Tests

  • mvn -pl agentscope-harness spotless:check test -Dtest=DockerSandboxCommandTest,BaseSandboxFilesystemTest,HarnessAgentDistributedSandboxTest -Dsurefire.failIfNoSpecifiedTests=false
    • 16 tests, 12 passed, 4 environment-gated skipped
  • AGENTSCOPE_DOCKER_INTEGRATION=true mvn -pl agentscope-harness test -Dtest=DockerSandboxCommandTest
    • 3/3 passed on Windows with Docker Desktop
    • covers nested quotes, $(dirname ...), paths with spaces, and a 100KB shell program
  • downstream JCode acceptance against the patched 2.0.1-SNAPSHOT
    • real Docker provider lifecycle 1/1
    • Redis + Docker fresh-JVM writer 1/1
    • Redis + Docker fresh-JVM reader 1/1

Closes #2135.
Addresses #1560.

@richuV richuV requested a review from a team July 12, 2026 01:04
@CLAassistant

CLAassistant commented Jul 12, 2026

Copy link
Copy Markdown

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.

u010143241 seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account.
You have signed the CLA already but the status is still pending? Let us recheck it.

This was referenced Jul 12, 2026
Open
Open
@codecov

codecov Bot commented Jul 12, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 27.27273% with 8 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
...rness/agent/sandbox/impl/docker/DockerSandbox.java 27.27% 8 Missing ⚠️

📢 Thoughts on this report? Let us know!

AgentScopeJavaBot
AgentScopeJavaBot approved these changes Jul 12, 2026
View reviewed changes

@AgentScopeJavaBot AgentScopeJavaBot left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤖 AI Review

Changes Docker sandbox command execution from sh -c to sh -s (stdin pipe), solving Windows command-line length limits, UTF-8 escaping issues, and process argument exposure. Well-scoped change (2 files, +108/-12) with comprehensive tests. Clean implementation, no issues found.

@AgentScopeJavaBot AgentScopeJavaBot added bug Something isn't working area/harness agentscope-harness (test/runtime support) labels Jul 12, 2026
@oss-maintainer

oss-maintainer commented Jul 14, 2026

Copy link
Copy Markdown
Collaborator

⚠️ CLA Not Signed

Please sign the CLA at: https://cla-assistant.io/agentscope-ai/agentscope-java?pullRequest=2148

Code Review (pending CLA)

Summary: Runs Docker sandbox shell programs through docker exec -i ... sh -s to avoid Windows command-line corruption and length limits.

Findings:

  • [Info] Keeps shell text out of host process argument list. Correct approach.
  • [Info] Preserves UTF-8 complex commands. Good.
  • [Info] 16 tests + real Docker regression. Excellent coverage.

Verdict: Essential fix for Windows Docker deployments. Ready to merge once CLA is signed.

Automated review by github-manager

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

1 more reviewer
@AgentScopeJavaBot AgentScopeJavaBot AgentScopeJavaBot approved these changes
Reviewers whose approvals may not affect merge requirements

At least 1 approving review is required to merge this pull request.

Assignees

No one assigned

Labels

area/harness agentscope-harness (test/runtime support) bug Something isn't working

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

[Bug]: DockerSandbox corrupts complex sh -c commands on Windows

4 participants

@richuV @CLAassistant @oss-maintainer @AgentScopeJavaBot